<html>
<head><meta charset="utf-8"><title>Reviewing the review process · t-compiler/rust-analyzer · Zulip Chat Archive</title></head>
<h2>Stream: <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/index.html">t-compiler/rust-analyzer</a></h2>
<h3>Topic: <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/Reviewing.20the.20review.20process.html">Reviewing the review process</a></h3>

<hr>

<base href="https://rust-lang.zulipchat.com">

<head><link href="https://rust-lang.github.io/zulip_archive/style.css" rel="stylesheet"></head>

<a name="199466887"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/Reviewing%20the%20review%20process/near/199466887" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> matklad <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/Reviewing.20the.20review.20process.html#199466887">(Jun 02 2020 at 09:18)</a>:</h4>
<p>Hey, so I feel we are suffering from a review bandwidth deficit. I want to help with this, by documenting more of the "internal invariants" and "how to do review" stuff. </p>
<p>I am putting info here: <a href="https://github.com/rust-analyzer/rust-analyzer/pull/4703">https://github.com/rust-analyzer/rust-analyzer/pull/4703</a></p>



<a name="200082800"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/Reviewing%20the%20review%20process/near/200082800" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> matklad <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/Reviewing.20the.20review.20process.html#200082800">(Jun 08 2020 at 11:04)</a>:</h4>
<p>Question: how do folks feel about reviewers pushing commits directly to the PR branch (allow edits from maintainers)?</p>
<p>I've noticed that the way I do thorough review is by checking out the code locally, and testing if my suggestions compile. So, in a sense, I first apply suggestions locally, and then write it again, as the comment on the PR. </p>
<p>I wonder if it makes sense to just directly push the suggestsions (preserving the same "motivation" part I would to GH comment to explain <em>why</em> this change is a good idea in my options)</p>



<a name="200086442"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/Reviewing%20the%20review%20process/near/200086442" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> vsrs <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/Reviewing.20the.20review.20process.html#200086442">(Jun 08 2020 at 11:50)</a>:</h4>
<p>LGTM</p>



<a name="200095537"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/Reviewing%20the%20review%20process/near/200095537" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Jeremy Kolb <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/Reviewing.20the.20review.20process.html#200095537">(Jun 08 2020 at 13:16)</a>:</h4>
<p>I think it should be allowed but would also say that I've learned a lot of rust from you telling me my PRs are wrong and having to type it all out myself :)</p>



<a name="200100104"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/Reviewing%20the%20review%20process/near/200100104" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> matklad <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/Reviewing.20the.20review.20process.html#200100104">(Jun 08 2020 at 13:54)</a>:</h4>
<p>Yeah, I guess, that's the main question -- would pedagogical value of reading someone else's commit be comparable to typing the stuff yourself...</p>



<a name="200101626"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/Reviewing%20the%20review%20process/near/200101626" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> bjorn3 <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/Reviewing.20the.20review.20process.html#200101626">(Jun 08 2020 at 14:05)</a>:</h4>
<p>When you type something yourself, you are forced to consciously think about it. When you read a commit you might just skim it without paying too much attention to the actual changes.</p>



<a name="200311870"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/Reviewing%20the%20review%20process/near/200311870" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> std::Veetaha <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/Reviewing.20the.20review.20process.html#200311870">(Jun 10 2020 at 01:42)</a>:</h4>
<p>I'd go for code snippet suggestions in PR itself. Pushing directly to PR branch you don't leave space for objections</p>



<a name="200311942"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/Reviewing%20the%20review%20process/near/200311942" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Joshua Nelson <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/Reviewing.20the.20review.20process.html#200311942">(Jun 10 2020 at 01:44)</a>:</h4>
<p>Code snippets are nice but I find that I'm tempted to make suggestions without testing them</p>



<a name="200311953"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/Reviewing%20the%20review%20process/near/200311953" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Joshua Nelson <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/Reviewing.20the.20review.20process.html#200311953">(Jun 10 2020 at 01:45)</a>:</h4>
<p>which I guess could be good pedagogically <span aria-label="laughing" class="emoji emoji-1f606" role="img" title="laughing">:laughing:</span> but also means I could suggest something that fundamentally doesn't work</p>



<hr><p>Last updated: Aug 07 2021 at 22:04 UTC</p>
</html>